Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update 2023 #58

Merged
merged 48 commits into from
Apr 9, 2024
Merged

Update 2023 #58

merged 48 commits into from
Apr 9, 2024

Conversation

kabu00002
Copy link
Collaborator

@kabu00002 kabu00002 commented Sep 15, 2023

Description

Update packages and adapt new changes from the KinaseFocusedFragmentLibrary update

Todos

Notable points that this PR has either accomplished or will accomplish.

Questions

Status

  • Ready to go

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

This was referenced Nov 9, 2023
Copy link
Collaborator

@PaulaKramer PaulaKramer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kabu00002! I have reviewed all files except the jupyter notebooks - I will do that in ReviewNB :)

README.md Show resolved Hide resolved
data/fragment_library_filtered/README.md Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
notebooks/figures/extreme_subpockets_one.png Outdated Show resolved Hide resolved
kinfraglib/utils.py Outdated Show resolved Hide resolved
Copy link

review-notebook-app bot commented Dec 20, 2023

View / edit / reply to this conversation on ReviewNB

PaulaKramer commented on 2023-12-20T16:22:17Z
----------------------------------------------------------------

This is missing the .to_html, if this is needed, check in all notebooks that it's consistent.


Copy link

review-notebook-app bot commented Dec 20, 2023

View / edit / reply to this conversation on ReviewNB

PaulaKramer commented on 2023-12-20T16:22:20Z
----------------------------------------------------------------

This list probably changed, we might have to look into that.


kabu00002 commented on 2024-01-24T14:57:00Z
----------------------------------------------------------------

There maybe something added but not deleted (?)

Copy link

review-notebook-app bot commented Dec 20, 2023

View / edit / reply to this conversation on ReviewNB

PaulaKramer commented on 2023-12-20T16:22:20Z
----------------------------------------------------------------

Check the warning, I think re-running the cell should already fix it.


Copy link
Collaborator Author

There maybe something added but not deleted (?)


View entire conversation on ReviewNB

Copy link
Collaborator

@dominiquesydow dominiquesydow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @PaulaKramer and @AndreaVolkamer,

Thanks a lot for this update!!

I may have missed this in one of the files - could you please add prominently to the README of this repo the timestamp of the dataset that you used for this update?

Which rdkit and Python version did you use to run this?
From the env file it seems you used python=3.8 and rdkit=2020.03.3

  • Re rdkit: Is there a reason for using the quite old version? If so, I would add that as a comment to the env file.
  • Re python: For the next update, we should move to the latest stable Python version (3.8 is almost deprecated).

I did not go through every notebook, do you have any specific questions regarding the notebooks? Happy to take a deep dive in that case.

@dominiquesydow
Copy link
Collaborator

@AndreaVolkamer I would drop Python 3.7 in the CI and only focus on 3.8 (as it is the Python version you used for the update afaiu).

@PaulaKramer
Copy link
Collaborator

Hi @dominiquesydow, thanks a lot for your comments!

We used rdkit=2021.09 (there were some changes with the dummy atom labels in the newer versions if I remember correctly, so we used the older version - @kabu00002 maybe you can add a comment regarding that). The rdkit version should be the same as in the KFFL repository.

I started working on the CI here: #62 (I haven't merged it yet). I removed python 3.7 and included 3.8 and 3.9 for ubuntu and mac, which seems to work. The CI is currently still failing for a few notebooks because the new combinatorial library is not on zenodo yet.

@PaulaKramer PaulaKramer mentioned this pull request Mar 26, 2024
7 tasks
@PaulaKramer
Copy link
Collaborator

Hi @dominiquesydow,
we adapted the readmes and updated the CI in case you would like to have another look before we merge it :)

Copy link
Collaborator

@dominiquesydow dominiquesydow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @PaulaKramer, many thanks for your work on this update — this looks great! :)

@PaulaKramer PaulaKramer merged commit ba07c3a into dev Apr 9, 2024
2 checks passed
@PaulaKramer PaulaKramer deleted the update_2023 branch April 9, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KinFragLib dataset update Add CITATION.cff List of publications using KinFragLib data
4 participants